Skip to content

Add per-route forward-health-checks option#246

Open
kemko wants to merge 4 commits intoumputun:masterfrom
kemko:forward-health-checks
Open

Add per-route forward-health-checks option#246
kemko wants to merge 4 commits intoumputun:masterfrom
kemko:forward-health-checks

Conversation

@kemko
Copy link

@kemko kemko commented Mar 3, 2026

Summary

Some HTTP services behind reproxy expose their own /ping or /health endpoints with application-specific responses that clients depend on. For example, audiobookshelf's /ping must return {"success": true} as a JSON object — its clients use this to verify the server is reachable and functioning correctly. Currently, reproxy's built-in pingHandler and healthMiddleware intercept all GET /ping and GET /health requests globally (responding with "pong" and a health JSON summary respectively) before route matching even runs, so these requests never reach the backend. There is no way to disable this per route or per domain.

This PR adds a new per-route option forward-health-checks (supported across all providers: Docker labels, file config, Consul Catalog tags, and static rules) that forwards /ping and /health requests to the backend instead of reproxy handling them. The option is off by default, so existing behavior is fully preserved.

Docker example

audiobookshelf:
  image: ghcr.io/advplyr/audiobookshelf
  labels:
    - "reproxy.server=audiobookshelf.example.com"
    - "reproxy.route=^/(.*)"
    - "reproxy.dest=/$1"
    - "reproxy.forward-health-checks"

File provider example

audiobookshelf.example.com:
  - { route: "^/(.*)", dest: "http://192.168.1.10:8080/$1", forward-health-checks: true }

Test plan

  • All existing tests pass (go test -race -timeout=60s -count 1 ./...)
  • New tests for Docker provider with forward-health-checks label
  • New tests for Consul Catalog provider with forward-health-checks tag
  • Updated file provider tests with forward-health-checks YAML field
  • New static provider tests for 5th element parsing
  • New proxy tests: /ping and /health forwarded when flag is set, built-in response preserved when not

@kemko kemko requested a review from umputun as a code owner March 3, 2026 09:10
@kemko
Copy link
Author

kemko commented Mar 11, 2026

Is there anything I can do to prevent this PR from being considered junk? Maybe I should file an issue?

@umputun
Copy link
Owner

umputun commented Mar 11, 2026

Is there anything I can do to prevent this PR from being considered junk? Maybe I should file an issue?

Hmm, why? I have seen it, noticed 15 updated files, and decided to postpone the review until I have time for this.

umputun
umputun requested changes Mar 11, 2026
View reviewed changes
Copy link
Owner

@umputun umputun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the branch is a bit behind master (missing #245 and #247), would be good to rebase for a clean diff.

main issue I see — inconsistent forward-health-checks value parsing across providers:

  • docker (docker.go:176-178): key presence alone enables it, so reproxy.forward-health-checks=false would still enable forwarding
  • static (static.go:40): any non-empty string enables it, same problem with "false" or "no"
  • consul-catalog (consulcatalog.go:195): properly checks for "true", "yes", "1"
  • file (file.go:88): YAML boolean, works correctly

docker should follow the existing keep-host pattern which inspects the value. static should also handle "false"/"no" correctly.

minor: shouldForwardHealthChecks in health.go calls Match() on every /ping and /health request, then matchHandler calls it again later for the same request. not a big deal given health check frequency, just noting it.

kemko added 4 commits March 15, 2026 14:31
@kemko
add ForwardHealthChecks field to URLMapper and all providers
727457a 
Add a new per-route boolean option `forward-health-checks` to URLMapper,
parsed by all providers (docker, file, consul-catalog, static).
When enabled, reproxy will forward /ping and /health requests to the
backend instead of handling them with built-in responses.

Made-with: Cursor
@kemko
forward /ping and /health to backend for routes with forward-health-c…
ec3b320 
…hecks

Modify pingHandler and healthMiddleware to check if the matched route has
ForwardHealthChecks enabled. If so, the request is forwarded to the backend
instead of being intercepted by reproxy's built-in responses.

Made-with: Cursor
@kemko
fixed linter issues
69adaa6
@kemko
fix inconsistent forward-health-checks value parsing across providers
70f0c70 
Docker now inspects the label value (true/yes/y/1) instead of just
checking key presence. Static provider checks for explicit positive
values instead of treating any non-empty string as truthy.

Made-with: Cursor
@kemko kemko force-pushed the forward-health-checks branch from 1de8384 to 70f0c70 Compare March 15, 2026 12:05
@kemko
Copy link
Author

kemko commented Mar 15, 2026

Rebased onto master (includes #245 and #247).

Fixed inconsistent value parsing — all providers now properly validate forward-health-checks values, matching existing conventions per provider:

  • Docker: extracted getForwardHealthChecksValue helper following the getKeepHostValue pattern — accepts true/yes/y/1, rejects false/no/n/0, warns on invalid values.
  • Static: checks for explicit positive values (true/yes/y/1) instead of treating any non-empty string as truthy. No warning on invalid values — consistent with how static handles other options.
  • Consul catalog: switched to the same switch pattern used by keep-host right above — explicit assignment for both true and false branches, warns on unrecognized values.
  • File: unchanged (YAML bool, already correct).

Added test cases for forward-health-checks=false in docker and static providers.

Re: double Match() in shouldForwardHealthChecks — reviewed, leaving as-is. The only way to avoid it is passing pre-computed matches via request context from health/ping middleware to matchHandler, which introduces implicit coupling between previously independent middleware and adds a context lookup on every request. Not worth the trade-off for a call that only affects forwarded health checks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@umputun umputun umputun requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kemko @umputun